feat(benchmark): Statistics rigor: implement pre-registered Holm correction + clamp the Wald risk-difference CI#22
Merged
Conversation
A paired risk difference is bounded [-1, 1], but the Wald normal approximation can spill past that (b large, c=0, small n gave an upper bound ~1.09). The CI is now clipped to the parameter space so it never reports an impossible rate difference. wilson_ci is already clamped and the odds ratio is intentionally unbounded, so only this CI needed it. No schema change (interval is unbounded); results-neutral.
The analysis plan (amendment-1) and paper method section both promise
Holm correction for the H1 secondary cells, but no code computed an
adjusted p-value. Adds:
- holm_adjust(): pure Holm-Bonferroni step-down (stdlib, deterministic)
- annotate_holm_family(): tags a crossover stats block per the frozen
pre-registration -- the rac-vs-naive_rag adherent cells at N in {50,150}
get p_value_holm + family='secondary' (Holm across whichever are
present); the N=300 confirmatory cell is tagged family='confirmatory'
and left UNcorrected (the single pre-registered primary test);
everything else is untouched (exploratory).
- crossover.py routes its stats block through finalize_crossover_stats()
at all three sites so single-seed, multi-seed, and merged datasets carry
the annotation.
- schema gains optional p_value_holm + family on the mcnemar object.
Results-neutral: adherence/cells/paired unchanged; only additive mcnemar
keys.
The paired-significance table gains a 'Holm p / family' column: secondary
cells show the Holm-adjusted p, the N=300 confirmatory cell reads
'confirmatory (uncorr.)', and exploratory cells show '—'. The section
gains a multiple-comparisons note stating exactly which cells are
corrected vs uncorrected (per the pre-registration), and the confirmatory
sentence is now labelled '(uncorrected)' with a companion line
summarizing the Holm-corrected secondary N in {50,150}. The paper's
stats_table.tex gets the matching Holm column + footnote, so the table
finally reflects method.tex's Holm claim.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two statistical-rigor gaps from review of
scoring/stats.py. Both are analysis-layer and results-neutral for adherence — verifiedarms/cells/per_scenario/pairedbyte-identical before/after; onlymcnemargains additive keys and the Wald CI is clipped to its valid range. Full suite green (471 passed, 6 skipped).1. Holm correction — promised but never implemented
spec/analysis-plan-amendment-1.mdandpaper/sections/method.texboth pre-register Holm correction for the H1 secondary cells, but no code computed an adjusted p-value — every McNemar p was rendered raw with no family-wise error control. This PR implements exactly what's pre-registered, no more:holm_adjust()— pure Holm-Bonferroni step-down (stdlib, deterministic).annotate_holm_family()— tags a crossoverstatsblock per the frozen pre-registration:rac-vs-naive_ragadherent at N∈{50,150} →p_value_holm+family="secondary"(Holm across whichever are present);family="confirmatory", left uncorrected (the single pre-registered primary test);crossover.pyroutes its stats block throughfinalize_crossover_stats()at all three sites (single-seed, multi-seed, merged).p_value_holm+familyon themcnemarobject.Holm p / familycolumn (secondary shows the Holm p, confirmatory reads "confirmatory (uncorr.)", exploratory shows "—"), a multiple-comparisons note, and the confirmatory sentence is now labelled "(uncorrected)" with a companion Holm summary for N∈{50,150}. The paper'sstats_table.texgets the matching column + footnote, so the table finally reflects method.tex's Holm claim.Scope note: the family is exactly the pre-registered one — deviating to a broader post-hoc Holm would itself contradict the frozen plan. The H2
passedsecondary (gitchameleon resolution path) is a separate family, an easy follow-on if wanted.2. Wald risk-difference CI wasn't clamped
risk_difference(b, c, n)returned[diff ± 1.96·se]raw. A risk difference is bounded [−1, 1], but the Wald normal approximation can spill past it (risk_difference(9, 0, 10)→ upper ≈ 1.09). Now clipped to the parameter space, so it never reports an impossible rate difference.wilson_ciwas already clamped and the odds ratio is intentionally unbounded, so only this CI needed it. No schema change (theintervaltype is unbounded).Testing
holm_adjuston hand-computed vectors (m=1 identity, cap at 1, monotonicity, order preserved);annotate_holm_familycorrectly corrects the secondary family, leaves the confirmatory cell uncorrected, ignores non-confirmatory pairs, and handles partial N grids; a Holm-annotated crossover block validates against the schema.risk_differenceclamp on out-of-range inputs (upper → 1.0, lower → −1.0); the existing in-range case still exact.Independent of PR #20 (roster Step 1); branched from
main.